[Icebox] Disperse GRPO app function/classes into respective files #137
Conversation
meta-cla
bot
added
the
CLA Signed
| logger.setLevel(logging.DEBUG) | ||
|
|
||
|
|
||
| def compute_logprobs( |
There was a problem hiding this comment.
Note that the implementation here is functionally equivalent to the implementation previously in reference_actor.py (which was moved to trainer.py)
| return logprobs | ||
|
|
||
|
|
||
| @dataclass |
There was a problem hiding this comment.
IMO keep this top level in grpo/main, at the very least don't put it in the trainer file
There was a problem hiding this comment.
Thoughts on where to store Episode down the line?
I'm fine using "Episode" before then
|
I don't actually think we should do this yet. I think we should just delete these as we have importable versions. I don't want the HF version to disperse into the codebase and I don't want us to abstract over different backends. We should be extremely lazy about moving things out of main. |
|
Quick responses! Noted, iceboxing this |
Jack-Khuu
changed the title
3 participants
There are no functional change
This PR does some sanitization and moves some the function/classes in
app/grpointo more appropriate files. Specifically:reference_agent.py=>reference_model.pyreference_agent.TitanRefModel=>reference_model.ReferenceModelgrpo.RefModel=>reference_model.HFReferenceModelcompute_logits+Episode=>trainer.pySpinning this up as a separate PR to make reviewing functional changes to RefModel clearer down the line